hotfix: fetch retry fix #8615
hotfix: fetch retry fix #8615
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
👋 Codeowner Review RequestThe following codeowners have been identified for the changed files: Team reviewers: @nodejs/nodejs-website Please review the changes when you have a chance. Thank you! 🙏 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8615 +/- ##
==========================================
+ Coverage 74.95% 75.02% +0.06%
==========================================
Files 103 103
Lines 9063 9068 +5
Branches 312 315 +3
==========================================
+ Hits 6793 6803 +10
+ Misses 2268 2263 -5
Partials 2 2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Pull request overview
Hotfix to prevent TypeScript-only utility imports from being used by non-TS (ESM .mjs) build/runtime scripts by relocating the fetchWithRetry helper into an ESM module and updating callers.
Changes:
- Removed the TypeScript
fetchWithRetryhelper (apps/site/util/fetch.ts). - Added an ESM-compatible retrying fetch helper (
apps/site/next.fetch.mjs). - Updated
.mjsmodules/generators to importfetchWithRetryfrom the new ESM module.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/site/util/fetch.ts | Removes the TS implementation to avoid TS imports from non-TS contexts. |
| apps/site/next.fetch.mjs | Introduces ESM fetchWithRetry implementation with retry/backoff options. |
| apps/site/next.calendar.mjs | Updates import to use the new ESM fetch helper. |
| apps/site/next-data/generators/vulnerabilities.mjs | Updates import to use the new ESM fetch helper. |
| apps/site/next-data/generators/supportersData.mjs | Updates import to use the new ESM fetch helper. |
| apps/site/next-data/generators/majorNodeReleases.mjs | Updates import to use the new ESM fetch helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const sleep = ms => new Promise(r => setTimeout(r, ms)); | ||
|
|
||
| /** | ||
| * Does a fetch with retry logic for network errors and timeouts. |
There was a problem hiding this comment.
The docstring says this retries on “network errors and timeouts”, but the implementation only retries when isTimeoutError(e) is true (ETIMEDOUT via e.cause.code). Either update the documentation to match current behavior or broaden the retry condition to include the intended network-error cases.
| * Does a fetch with retry logic for network errors and timeouts. | |
| * Does a fetch with retry logic for timeout errors (as determined by `isTimeoutError`). |
📦 Build Size ComparisonSummary
Changes➕ Added Assets (2)
➖ Removed Assets (2)
|
|
cc @nodejs/nodejs-website fast-tracking as urgent |
1 participant
This is a hot-fix due to a TS import happening on non-TS files...